Claude finished @carlos-alm's task —— View job

PR Review

• Read changed files and gather context
• Phase 1: Root cause analysis
• Phase 2: Configuration change detection
• Phase 3: Test integrity
• Phase 4: Code quality
• Phase 5: Backlog compliance
• Final assessment

Phase 1: Root Cause Analysis

Problem Being Solved: The INLINE_BACKFILL_THRESHOLD was set to 16, but the primary hot caller (runPostNativeThisDispatch) processes up to ~18 JS/TS files and was hitting the pool path unnecessarily. Measurements showed IPC overhead at ~55–64ms/file through the pool vs ~8–10ms/file inline — a ~3–6x penalty. The old docstring falsely implied pool overhead was "amortised over parse work," when in reality it scales linearly.

Why This Approach: Two orthogonal fixes in one minimal change — (1) raise the threshold past the measured batch size of 18, and (2) correct the misleading docstring to accurately explain the crash-safety rationale (not cost amortisation). The threshold of 32 gives headroom over 18 while still routing truly large batches through the pool.

Risk Assessment: Low. JS/TS/TSX are required-tier grammars with no history of the V8 fatal crash class. Raising the inline ceiling to 32 from 16 for files that call parseFilesWasmForBackfill means the only meaningful difference is performance. The exotic-language backfill path (e.g. HCL, ~4 files) benefits from inline too, with negligible additional crash risk.

Phase 2: Configuration Change Detection

No linting rules, test timeouts, or build configuration were changed. The only constant modified is INLINE_BACKFILL_THRESHOLD — a behavioral performance tuning parameter, not a quality gate. No concerns here.

Phase 3: Test Integrity

No tests were modified. There are no direct unit tests for parseFilesWasmForBackfill or INLINE_BACKFILL_THRESHOLD (confirmed via search) — the threshold is an internal constant exercised through integration tests. No assertions were weakened or removed.

Minor observation: the threshold is a magic constant with no test asserting the inline path is taken for expected batch sizes. This isn't a blocker, but a test that exercises parseFilesWasmForBackfill with a batch at the boundary (e.g. 31 vs 33 files) would prevent future regressions. Not required for this PR.

Phase 4: Code Quality

Docstring correctness (src/domain/parser.ts:1181–1201): The new docstring is materially more accurate:

• Correctly identifies crash safety (not amortisation) as the core rationale ✓
• Cites the specific measured numbers (55–64ms/file pool, 8–10ms/file inline) with issue reference ✓
• Explains why JS/TS/TSX are safe inline (required-tier, no V8 fatal crash history) ✓
• Acknowledges exotic drops also benefit from inline given they're typically ≤ 4 files ✓

Threshold value (src/domain/parser.ts:1202): 32 is well-justified. The measured batch is ≤ 18; 32 gives ~1.7x headroom. No realistic exotic-language scenario is expected to exceed 32 based on the documented HCL case being 4 files.

Secondary docstring (src/domain/parser.ts:1253–1265, parseFilesWasmForBackfill): This docstring still reads "Threshold matches typical engine-parity drop sizes (a few fixture files in one or two languages)" — that was accurate for the old 16 value but is now slightly misleading since the threshold was raised specifically to cover the larger this-dispatch batches, not just engine-parity drops. This is a minor inconsistency that could be addressed in a follow-up, but it's not incorrect.

No bugs, security issues, or architectural concerns introduced.

Phase 5: Backlog Compliance

• Zero-dep: ✓ — No new runtime dependencies
• Foundation-aligned: ✓ — Pure internal performance tuning; no cloud APIs, no external services, no agent behavior
• Problem-fit: 3/5 — Improves build performance (especially in native-engine post-pass), which indirectly speeds up graph generation for AI agents. Doesn't directly address token waste or hallucinated deps, but faster indexing is a meaningful quality-of-life improvement
• Breaking: No — internal constant, no API or CLI contract changes
• Tier: 1

Final Assessment

## Root Cause Analysis
Problem Being Solved: INLINE_BACKFILL_THRESHOLD=16 was below the measured
  batch size of ~18 JS/TS files in runPostNativeThisDispatch, causing a
  3–6x performance penalty from unnecessary IPC overhead. The old docstring
  also incorrectly described cost amortisation as the rationale.
Why This Approach: Raise to 32 (covers the 18-file batch with headroom),
  and correct the docstring to explain the real crash-safety motivation.
Risk Assessment: Low — JS/TS/TSX are required-tier with no V8 fatal crash
  history; raising the inline ceiling for them carries negligible crash risk.

## Backlog Compliance
- Zero-dep: ✓
- Foundation-aligned: ✓
- Problem-fit: 3/5 — performance improvement to graph indexing pipeline
- Breaking: No
- Tier: 1

## Minor Concerns
- src/domain/parser.ts:1253–1265: The `parseFilesWasmForBackfill` docstring
  still says "threshold matches typical engine-parity drop sizes" which is
  now somewhat misleading — the threshold was raised for this-dispatch batch
  coverage. Not blocking; can be addressed in follow-up.
- No boundary test exists for the threshold (31 vs 33 files). Not required
  but would prevent silent regressions.

## Final Recommendation
- Rating: ⭐⭐⭐⭐ (4/5)
- Action: APPROVE
- Reasoning: The change is well-motivated with cited measurements and a
  referenced issue (#1435). The docstring correction accurately replaces
  misleading "amortisation" framing with the true crash-safety rationale.
  The threshold value is justified by the measured batch size. Minor
  inconsistency in the secondary docstring is cosmetic and non-blocking.

Greptile Summary

This PR corrects the INLINE_BACKFILL_THRESHOLD docstring (from a misleading "IPC cost is amortised" rationale to the accurate crash-safety explanation) and raises the threshold from 16 to 32 to keep typical JS/TS/TSX this-dispatch batches on the faster inline path. Two incidental optimisations are also included in native-orchestrator.ts.

• parser.ts: Threshold doubled to 32; docstring now explains the V8 crash-isolation purpose of the worker pool, the safety of JS/TS/TSX inline, and the measured 3–6× IPC penalty that motivates the higher limit.
• native-orchestrator.ts: Backfill guard simplified to check only the gap arrays (safe — backfillNativeDroppedFiles already returns immediately on an empty gap); COUNT(*) re-count now skipped via postPassWroteData when no post-pass wrote new rows.

Confidence Score: 5/5

Safe to merge — the threshold change is a well-measured constant bump, and the native-orchestrator logic simplifications are guarded by the function's own early-return.

The threshold raise is backed by concrete measurements and narrows crash risk minimally (JS/TS/TSX have never triggered the V8 fatal class). The backfill-condition simplification is safe because backfillNativeDroppedFiles already returns immediately when both gap arrays are empty. The COUNT(*) fast path correctly excludes only backfillEdgeTechniquesAfterNativeOrchestrator, which updates an existing column rather than inserting new rows.

No files require special attention.

Important Files Changed

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[parseFilesWasmForBackfill\ncalled with N files] --> B{N ≤ INLINE_BACKFILL_THRESHOLD\nnow 32, was 16}
    B -- yes\nJS/TS/TSX this-dispatch\nbatches ≤18 files --> C[parseFilesWasmInline\nmain-thread ~8–10ms/file]
    B -- no\nlarge or exotic-language drops --> D[parseFilesWasm via worker pool\ncrash-isolated ~55–64ms/file]
    C --> E[Return results]
    D --> E

    subgraph native-orchestrator fast paths
        F[detectDroppedLanguageGap] --> G{gap empty?}
        G -- yes --> H[Skip backfillNativeDroppedFiles]
        G -- no --> I[backfillNativeDroppedFiles\nwrites nodes+edges]
        I --> J{postPassWroteData?\nbackfill OR CHA edges\nOR this-dispatch targets}
        J -- no --> K[Skip COUNT star scan\nRust counts still accurate]
        J -- yes --> L[COUNT star re-count\nupdate build_meta]
    end

_{Reviews (5): Last reviewed commit: "fix(perf): guard post-native passes on 1..." | Re-trigger Greptile}

Fixed — updated the stale parseFilesWasmForBackfill docstring at line 1256 to replace the old 16-value rationale ("engine-parity drop sizes") with a pointer to INLINE_BACKFILL_THRESHOLD where the full rationale now lives (f90d4ba).

@greptileai

Codegraph Impact Analysis

1 functions changed → 5 callers affected across 5 files

• tryNativeOrchestrator in src/domain/graph/builder/stages/native-orchestrator.ts:1384 (5 transitive callers)